Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

#4332: Fix import status dashboard pagination #4333

Open
wants to merge 1 commit into
base: 2.x
Choose a base branch
from
Open

#4332: Fix import status dashboard pagination #4333

wants to merge 1 commit into from

Conversation

stefan-korn
Copy link
Contributor

Fixes #4332

Describe your changes

Solution is maybe impacting performance, since all the datasets are requested before paging. But did not see how to fix this issue otherwise.

QA Steps

  • Add manual QA steps in checklist format for a reviewer to perform. Be as specific as possible, provide examples if appropriate.

Checklist before requesting review

If any of these are left unchecked, please provide an explanation

  • I have updated or added tests to cover my code
  • I have updated or added documentation

@dafeder dafeder self-requested a review November 21, 2024 22:28
@dafeder dafeder self-assigned this Nov 21, 2024
@paul-m
Copy link
Contributor

paul-m commented Jan 3, 2025 •
edited
Loading

Thanks!

At minimum, this needs a unit test of filterImportableDatasets() to prove that it does the right thing. This could just mock datasetInfo->gather() to return an info array that would be array_filter()'d to nothing.

Bonus after that is to create a dataset with nothing to import, and then show that it does not display on the status page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dafeder dafeder Awaiting requested review from dafeder

At least 1 approving review is required to merge this pull request.

Assignees

@dafeder dafeder

Labels
Needs test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Import status dashboard pagination issue - follow-up to #4317
3 participants
@stefan-korn @paul-m @dafeder